Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

modify resetall to work with patch object #241

Merged
merged 3 commits into from
May 6, 2021

Conversation

shadycuz
Copy link
Contributor

@shadycuz shadycuz commented May 5, 2021

Fixes #237

It seems spy and patch.object have a reset_mock that doesn't take the return_value and side_effect keyword args.

I originally tried to fix this in _start_patch

p = mock_func(*args, **kwargs)
mocked = p.start() # type: unittest.mock.MagicMock
self._patches.append(p)
if hasattr(mocked, "reset_mock"):
self._mocks.append(mocked)
# check if `mocked` is actually a mock object, as depending on autospec or target
# parameters `mocked` can be anything
if hasattr(mocked, "__enter__") and warn_on_mock_enter:

but changing self._mocks.append(mocked) to only append actually Mock objects breaks my repo worse because then I lose the ability to call resetall on patched objects.

This seemed like the best approach to take.

I also tried to get this to fail in a test case but could not get the error to trigger. I suspect the issue might be with the version of mock or unittest.mock that our projects are using. I should be on the lastest of everything, but I'm still facing the error which is strange 🤔

Here is my repo DontShaveTheYak/cloud-radar#32 if you want to try and debug it.

Comment on lines 313 to 315
# Testing spy can still be reset.
mocker.resetall()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a working test case. If you add this line to the previous version 3.6.0 then it triggers the error.

@shadycuz
Copy link
Contributor Author

shadycuz commented May 6, 2021

I now believe the failure happens when autospec = True but I don't know why 🤷‍♂️. That's the only thing in common between my failure with patch.object and the failure with spy. I think they both are using autospec. I read through the mock source code but couldn't find the reason.

@nicoddemus
Copy link
Member

I now believe the failure happens when autospec = True but I don't know why 🤷‍♂️.

Yeah autospec is a bit convoluted... IIRC we can't use autospec with classmethod (or staticmethod) for example.

@nicoddemus nicoddemus merged commit b4d91e0 into pytest-dev:main May 6, 2021
@nicoddemus
Copy link
Member

Thanks a lot @shadycuz, appreciate it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mocker.resetall() doesn't work anymore when using mocker.spy()
2 participants